Enhance Docker Compose and service management for LangFuse integration#306
Enhance Docker Compose and service management for LangFuse integration#306AnkushMalaker merged 1 commit intodevfrom
Conversation
- Updated `services.py` to include new options for service management, allowing for forced recreation of containers during startup. - Added LangFuse configuration options in the setup wizard, improving user experience for observability setup. - Introduced new API endpoints for retrieving observability configuration, enhancing integration with the frontend. - Enhanced error handling and logging for service startup processes, ensuring better visibility of configuration issues. - Updated documentation to reflect changes in service management and LangFuse integration.
📝 WalkthroughWalkthroughThis PR integrates Langfuse observability throughout the backend and frontend systems. It adds configuration persistence for public URLs, exposes observability endpoints, threads session identifiers through LLM calls for tracing, updates memory and conversation services, and augments the UI to display Langfuse links. Additionally, Docker service management gains force-recreate support. Changes
Sequence DiagramsequenceDiagram
participant Frontend as Frontend App
participant SystemAPI as System API
participant Controller as System Controller
participant LLMClient as LLM Client
participant Memory as Memory Service
participant OpenAI as OpenAI API
participant LangFuse as Langfuse
Frontend->>SystemAPI: GET /api/observability
SystemAPI->>Controller: get_observability_config()
Controller->>Controller: Load config & derive session_base_url
Controller-->>SystemAPI: {langfuse.enabled, session_base_url}
SystemAPI-->>Frontend: Observability config
Frontend->>Frontend: Store langfuseSessionUrl
Frontend->>Frontend: Render Langfuse link with session URL
Note over Frontend,LangFuse: When processing conversation
Frontend->>SystemAPI: Process conversation (conversation_id)
SystemAPI->>Memory: extract_memories(langfuse_session_id=conversation_id)
Memory->>LLMClient: generate(langfuse_kwargs with metadata)
LLMClient->>LLMClient: _langfuse_metadata(session_id)
LLMClient->>OpenAI: chat.completions.create(metadata={...})
OpenAI->>LangFuse: Trace with session context
LangFuse-->>OpenAI: Acknowledgment
OpenAI-->>LLMClient: Response
LLMClient-->>Memory: Generated content
Memory-->>SystemAPI: Memory update result
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes 🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
backends/advanced/src/advanced_omi_backend/services/memory/providers/llm_providers.py (1)
267-274:⚠️ Potential issue | 🟠 MajorUnconditional
metadata={}may cause API failures on some gateways when Langfuse is disabled or session_id is absent
_langfuse_metadata()returns{}whenever Langfuse is not enabled orsession_idisNone. The code unconditionally passesmetadata=_langfuse_metadata(...)to the OpenAI client at all call sites. Some OpenAI-compatible gateways reject the presence ofmetadata={}(empty dict) and return a 400 error. The recommended pattern is to omit themetadataparameter entirely when it is empty, rather than passing an explicit empty dict.This pattern appears across both async and blocking code paths:
llm_providers.pylines 267, 358, 444, andllm_client.pylines 236, 273 (inasync_generate()andasync_chat_with_tools()).🐛 Proposed fix — apply at all five call sites
- response = await client.chat.completions.create( - **op.to_api_params(), - messages=[...], - metadata=_langfuse_metadata(langfuse_session_id), - ) + create_kwargs = {**op.to_api_params(), "messages": [...]} + _meta = _langfuse_metadata(langfuse_session_id) + if _meta: + create_kwargs["metadata"] = _meta + response = await client.chat.completions.create(**create_kwargs)Apply the same guard at
llm_providers.pylines 358 and 444, andllm_client.pylines 236 and 273.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backends/advanced/src/advanced_omi_backend/services/memory/providers/llm_providers.py` around lines 267 - 274, The call sites currently pass metadata=_langfuse_metadata(...) unconditionally (e.g., in client.chat.completions.create in llm_providers.py and in async_generate()/async_chat_with_tools() in llm_client.py); change each site to only include the metadata kwarg when _langfuse_metadata(...) returns a non-empty dict — i.e., compute meta = _langfuse_metadata(...) and pass metadata=meta only if meta, otherwise omit the metadata parameter so empty {} is not sent to gateways that reject it; apply the same guard at the other occurrences referenced (llm_providers.py lines near the client.chat.completions.create calls and llm_client.py async_generate/async_chat_with_tools).
🧹 Nitpick comments (5)
config/defaults.yml (1)
506-515: Moveobservability:block out from between the Cron Jobs header and its key.The new block is inserted between the
# Cron Jobs Configurationcomment header (lines 503–505) and thecron_jobs:mapping (line 516), visually orphaning that section header from the data it describes. While YAML parsing is unordered and this won't break OmegaConf loading, it makes the file misleading at a glance.♻️ Suggested placement — move `observability` before the Cron Jobs section
+# =========================== +# Observability Configuration +# =========================== +observability: + langfuse: + # Browser-accessible URL for linking to Langfuse sessions. + # Distinct from LANGFUSE_HOST which may be an internal Docker URL. + # Example: http://localhost:3002 + public_url: '' + # =========================== # Cron Jobs Configuration # =========================== -# =========================== -# Observability Configuration -# =========================== -observability: - langfuse: - # Browser-accessible URL for linking to Langfuse sessions. - # Distinct from LANGFUSE_HOST which may be an internal Docker URL. - # Example: http://localhost:3002 - public_url: '' - cron_jobs:🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@config/defaults.yml` around lines 506 - 515, The observability block (observability: / langfuse: / public_url:) has been inserted between the "# Cron Jobs Configuration" header and the cron_jobs: mapping which visually separates the header from its section; move the entire observability: block (including langfuse.public_url) up so it appears immediately before the Cron Jobs section header, ensuring the "# Cron Jobs Configuration" comment is followed directly by the cron_jobs: mapping.services.py (1)
291-292: Use iterable unpacking instead of list concatenation (Ruff RUF005).Static analysis flags lines 291 and 332. Line 329 has the same pattern and should be updated for consistency.
♻️ Proposed fix
- cmd.extend(up_flags + ['speaker-service-gpu' if profile == 'gpu' else 'speaker-service-cpu', 'web-ui']) + cmd.extend([*up_flags, 'speaker-service-gpu' if profile == 'gpu' else 'speaker-service-cpu', 'web-ui'])- cmd.extend(up_flags + services_to_start) + cmd.extend([*up_flags, *services_to_start])- cmd.extend(up_flags + ['vibevoice-asr']) + cmd.extend([*up_flags, 'vibevoice-asr'])Also applies to: 329-332
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@services.py` around lines 291 - 292, Replace list concatenation inside cmd.extend calls with iterable unpacking or separate extends: find the cmd.extend(up_flags + [...]) usage (the occurrences around the block that chooses 'speaker-service-gpu' if profile == 'gpu' else 'speaker-service-cpu' and 'web-ui') and change it to either cmd.extend([*up_flags, 'speaker-service-gpu' if profile == 'gpu' else 'speaker-service-cpu', 'web-ui']) or split into two calls (cmd.extend(up_flags); cmd.extend(['speaker-service-gpu' if profile == 'gpu' else 'speaker-service-cpu', 'web-ui'])); apply the same change to the other occurrence around lines 329-332 to remove list concatenation per Ruff RUF005.backends/advanced/src/advanced_omi_backend/services/memory/base.py (1)
372-396:langfuse_session_idnot documented inpropose_memory_actionsdocstring
extract_memorieshas its new parameter documented at line 353, but theArgssection ofpropose_memory_actions(andpropose_reprocess_actionsat line 414–422) was not updated to includelangfuse_session_id, creating an inconsistency in the interface documentation.📝 Proposed fix
Args: retrieved_old_memory: List of existing memories for context new_facts: List of new facts to process custom_prompt: Optional custom prompt to use instead of default + langfuse_session_id: Optional session ID for Langfuse trace grouping Returns:Apply the same addition to the
propose_reprocess_actionsdocstring Args block.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backends/advanced/src/advanced_omi_backend/services/memory/base.py` around lines 372 - 396, The docstring for propose_memory_actions is missing the langfuse_session_id parameter documentation — update the Args section of propose_memory_actions (and likewise propose_reprocess_actions) to document langfuse_session_id: its type Optional[str], purpose as the Langfuse tracing/session identifier, and that it’s optional and passed through for observability; ensure the doc entries match the style and wording used for extract_memories’s new parameter to keep interface docs consistent.backends/advanced/src/advanced_omi_backend/llm_client.py (2)
199-203:_langfuse_metadatais duplicated — consolidate intoopenai_factory.pyAn identical
_langfuse_metadatahelper is defined in both this file (lines 199–203) andllm_providers.py(lines 41–45). Both files already import fromadvanced_omi_backend.openai_factory, making it the natural home for this shared utility.♻️ Proposed refactor
In
openai_factory.py, add:def _langfuse_metadata(session_id: str | None) -> dict: """Return metadata dict with langfuse_session_id if Langfuse is enabled.""" if session_id and is_langfuse_enabled(): return {"langfuse_session_id": session_id} return {}Then in both
llm_client.pyandllm_providers.py, replace the local definition with:-from advanced_omi_backend.openai_factory import create_openai_client, is_langfuse_enabled +from advanced_omi_backend.openai_factory import create_openai_client, is_langfuse_enabled, _langfuse_metadataand remove the local
_langfuse_metadatadefinitions.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backends/advanced/src/advanced_omi_backend/llm_client.py` around lines 199 - 203, Consolidate the duplicated helper by moving the _langfuse_metadata function into advanced_omi_backend.openai_factory (add the function exactly as in the diff) and update llm_client.py and llm_providers.py to import and call openai_factory._langfuse_metadata instead of defining their own copies; then remove the local _langfuse_metadata definitions from both files so there is a single shared implementation to maintain.
243-245:langfuse_session_idis silently dropped on the singleton-client fallback pathWhen
operationisNone(or the registry is absent),async_generateandasync_chat_with_toolsfall back to the singleton client without forwardinglangfuse_session_id. All session tracing is lost for callers that rely on the fallback path, creating inconsistent observability coverage.♻️ Proposed fix
# Fallback: use singleton client client = get_llm_client() loop = asyncio.get_running_loop() -return await loop.run_in_executor( - None, lambda: client.generate(prompt, model, temperature) -) +langfuse_kwargs = _langfuse_metadata(langfuse_session_id) +return await loop.run_in_executor( + None, lambda: client.generate(prompt, model, temperature, **langfuse_kwargs) +)Apply the same fix to the
async_chat_with_toolsfallback at lines 279–281.Also applies to: 279-281
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backends/advanced/src/advanced_omi_backend/llm_client.py` around lines 243 - 245, The fallback branch in async_generate (and likewise in async_chat_with_tools) uses run_in_executor with lambda: client.generate(prompt, model, temperature) which drops langfuse_session_id; update both fallback calls to pass the langfuse_session_id through to client.generate and client.chat_with_tools (i.e., include the session_id kwarg or parameter name used by the client) inside the lambda so the singleton-client path preserves tracing; modify the lambda in async_generate and the similar lambda in async_chat_with_tools to forward the langfuse_session_id argument.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@backends/advanced/webui/src/pages/ConversationDetail.tsx`:
- Around line 604-614: The href construction can produce a double slash when
langfuseSessionUrl has a trailing slash; in ConversationDetail.tsx update the
JSX that builds href
(`href={`${langfuseSessionUrl}/${conversation.conversation_id}`}`) to normalize
the base URL first—either strip any trailing '/' from langfuseSessionUrl before
concatenation or use a URL-joining approach (e.g., new
URL(conversation.conversation_id, langfuseSessionUrl)) so the final href
reliably becomes "<base>/<conversation_id>" without duplicate slashes.
---
Outside diff comments:
In
`@backends/advanced/src/advanced_omi_backend/services/memory/providers/llm_providers.py`:
- Around line 267-274: The call sites currently pass
metadata=_langfuse_metadata(...) unconditionally (e.g., in
client.chat.completions.create in llm_providers.py and in
async_generate()/async_chat_with_tools() in llm_client.py); change each site to
only include the metadata kwarg when _langfuse_metadata(...) returns a non-empty
dict — i.e., compute meta = _langfuse_metadata(...) and pass metadata=meta only
if meta, otherwise omit the metadata parameter so empty {} is not sent to
gateways that reject it; apply the same guard at the other occurrences
referenced (llm_providers.py lines near the client.chat.completions.create calls
and llm_client.py async_generate/async_chat_with_tools).
---
Nitpick comments:
In `@backends/advanced/src/advanced_omi_backend/llm_client.py`:
- Around line 199-203: Consolidate the duplicated helper by moving the
_langfuse_metadata function into advanced_omi_backend.openai_factory (add the
function exactly as in the diff) and update llm_client.py and llm_providers.py
to import and call openai_factory._langfuse_metadata instead of defining their
own copies; then remove the local _langfuse_metadata definitions from both files
so there is a single shared implementation to maintain.
- Around line 243-245: The fallback branch in async_generate (and likewise in
async_chat_with_tools) uses run_in_executor with lambda: client.generate(prompt,
model, temperature) which drops langfuse_session_id; update both fallback calls
to pass the langfuse_session_id through to client.generate and
client.chat_with_tools (i.e., include the session_id kwarg or parameter name
used by the client) inside the lambda so the singleton-client path preserves
tracing; modify the lambda in async_generate and the similar lambda in
async_chat_with_tools to forward the langfuse_session_id argument.
In `@backends/advanced/src/advanced_omi_backend/services/memory/base.py`:
- Around line 372-396: The docstring for propose_memory_actions is missing the
langfuse_session_id parameter documentation — update the Args section of
propose_memory_actions (and likewise propose_reprocess_actions) to document
langfuse_session_id: its type Optional[str], purpose as the Langfuse
tracing/session identifier, and that it’s optional and passed through for
observability; ensure the doc entries match the style and wording used for
extract_memories’s new parameter to keep interface docs consistent.
In `@config/defaults.yml`:
- Around line 506-515: The observability block (observability: / langfuse: /
public_url:) has been inserted between the "# Cron Jobs Configuration" header
and the cron_jobs: mapping which visually separates the header from its section;
move the entire observability: block (including langfuse.public_url) up so it
appears immediately before the Cron Jobs section header, ensuring the "# Cron
Jobs Configuration" comment is followed directly by the cron_jobs: mapping.
In `@services.py`:
- Around line 291-292: Replace list concatenation inside cmd.extend calls with
iterable unpacking or separate extends: find the cmd.extend(up_flags + [...])
usage (the occurrences around the block that chooses 'speaker-service-gpu' if
profile == 'gpu' else 'speaker-service-cpu' and 'web-ui') and change it to
either cmd.extend([*up_flags, 'speaker-service-gpu' if profile == 'gpu' else
'speaker-service-cpu', 'web-ui']) or split into two calls (cmd.extend(up_flags);
cmd.extend(['speaker-service-gpu' if profile == 'gpu' else
'speaker-service-cpu', 'web-ui'])); apply the same change to the other
occurrence around lines 329-332 to remove list concatenation per Ruff RUF005.
| {langfuseSessionUrl && conversation.conversation_id && ( | ||
| <a | ||
| href={`${langfuseSessionUrl}/${conversation.conversation_id}`} | ||
| target="_blank" | ||
| rel="noopener noreferrer" | ||
| className="p-2 rounded-full hover:bg-blue-100 dark:hover:bg-blue-900/30 transition-colors" | ||
| title="View traces in Langfuse" | ||
| > | ||
| <BarChart3 className="h-5 w-5 text-gray-400 dark:text-gray-500 hover:text-blue-500 dark:hover:text-blue-400" /> | ||
| </a> | ||
| )} |
There was a problem hiding this comment.
Potential double slash in the Langfuse trace URL
langfuseSessionUrl stores cfg.session_base_url verbatim. If the backend serves this value with a trailing slash (e.g., https://langfuse.example.com/sessions/), the constructed URL becomes https://langfuse.example.com/sessions//conversation-id, which most observability UIs will not resolve correctly.
🐛 Proposed fix
- href={`${langfuseSessionUrl}/${conversation.conversation_id}`}
+ href={`${langfuseSessionUrl.replace(/\/$/, '')}/${conversation.conversation_id}`}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| {langfuseSessionUrl && conversation.conversation_id && ( | |
| <a | |
| href={`${langfuseSessionUrl}/${conversation.conversation_id}`} | |
| target="_blank" | |
| rel="noopener noreferrer" | |
| className="p-2 rounded-full hover:bg-blue-100 dark:hover:bg-blue-900/30 transition-colors" | |
| title="View traces in Langfuse" | |
| > | |
| <BarChart3 className="h-5 w-5 text-gray-400 dark:text-gray-500 hover:text-blue-500 dark:hover:text-blue-400" /> | |
| </a> | |
| )} | |
| {langfuseSessionUrl && conversation.conversation_id && ( | |
| <a | |
| href={`${langfuseSessionUrl.replace(/\/$/, '')}/${conversation.conversation_id}`} | |
| target="_blank" | |
| rel="noopener noreferrer" | |
| className="p-2 rounded-full hover:bg-blue-100 dark:hover:bg-blue-900/30 transition-colors" | |
| title="View traces in Langfuse" | |
| > | |
| <BarChart3 className="h-5 w-5 text-gray-400 dark:text-gray-500 hover:text-blue-500 dark:hover:text-blue-400" /> | |
| </a> | |
| )} |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@backends/advanced/webui/src/pages/ConversationDetail.tsx` around lines 604 -
614, The href construction can produce a double slash when langfuseSessionUrl
has a trailing slash; in ConversationDetail.tsx update the JSX that builds href
(`href={`${langfuseSessionUrl}/${conversation.conversation_id}`}`) to normalize
the base URL first—either strip any trailing '/' from langfuseSessionUrl before
concatenation or use a URL-joining approach (e.g., new
URL(conversation.conversation_id, langfuseSessionUrl)) so the final href
reliably becomes "<base>/<conversation_id>" without duplicate slashes.
services.pyto include new options for service management, allowing for forced recreation of containers during startup.Summary by CodeRabbit
New Features
Documentation